-
Notifications
You must be signed in to change notification settings - Fork 23
Unify dpnp public API exports by refactoring __init__.py
#2666
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: refactor_public_api_exports
Are you sure you want to change the base?
Unify dpnp public API exports by refactoring __init__.py
#2666
Conversation
|
View rendered docs @ https://intelpython.github.io/dpnp/pull/2666/index.html |
|
Array API standard conformance tests for dpnp=0.20.0dev0=py313h509198e_69 ran successfully. |
| * Redesigned `dpnp.modf` function to be a part of `ufunc` and `vm` pybind11 extensions [#2654](https://github.com/IntelPython/dpnp/pull/2654) | ||
| * Refactored `dpnp.fft` and `dpnp.random` submodules by removing wildcard imports and defining explicit public exports [#2649](https://github.com/IntelPython/dpnp/pull/2649) | ||
| * Added support for the `out` keyword to accept a tuple, bringing ufunc signatures into alignment with those in NumPy [#2664](https://github.com/IntelPython/dpnp/pull/2664) | ||
| * Unified `dpnp` public API exports by consolidating function exports in `__init__.py` and removing wildcard imports [#2665](https://github.com/IntelPython/dpnp/pull/2665) [#2666](https://github.com/IntelPython/dpnp/pull/2666) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove "--disable=c-extension-no-member" from .pre-commit-config.yaml now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And E501 skipping in .flake8
| from .dpnp_iface_types import ( | ||
| e, | ||
| euler_gamma, | ||
| inf, | ||
| nan, | ||
| newaxis, | ||
| pi, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It has to be a part of the below Routines
| ) | ||
|
|
||
| # ----------------------------------------------------------------------------- | ||
| # Constants (borrowed from NumPy) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| # Constants (borrowed from NumPy) | |
| # Constants |
| # The order of these declarations are borrowed from the NumPy document: | ||
| # https://numpy.org/doc/stable/reference/arrays.scalars.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does not really make any sense, because there is a single from .dpnp_iface_types import below which is connected with that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The order is defined by the python sort linter and not by the pointing numpy's page.
| # ----------------------------------------------------------------------------- | ||
| # Type-related helper functions | ||
| # ----------------------------------------------------------------------------- | ||
| from .dpnp_iface_types import ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a part of Data type routines from https://numpy.org/doc/stable/reference/routines.dtype.html.
Why isn't it moved below to the routines?
| iinfo, | ||
| isdtype, | ||
| issubdtype, | ||
| is_type_supported, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That needs to be moved away. It's a kind of deprecated API which we should not expose.
Probably it would be better to move that to dpnp/random/dpnp_iface_random.py (where it is only used) and to make it an internal function.
| # ----------------------------------------------------------------------------- | ||
| from .dpnp_iface_types import ( | ||
| common_type, | ||
| finfo, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to move dtype here instead of the above line#97 ?
| isdtype, | ||
| issubdtype, | ||
| is_type_supported, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems result_type has be a part of that block, if we are following the routines page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And can_cast also
This PR removes the remaining wildcard imports and
__all__declarations from internal modules and consolidates the corresponding exports indpnp/__init__.pyas part of the ongoing public API unification.This work follows #2665.